Updated semantics for atproto labelers header #2292
Conversation
|
|
||
| const makeResHeaders = (res: Response): Record<string, string> => { | ||
| const headers = RES_HEADERS_TO_FORWARD.reduce((acc, cur) => { | ||
| acc[cur] = res.headers.get(cur) ?? undefined |
There was a problem hiding this comment.
Is it possible for headers.get(cur) to be null? That is the only case I can imagine the ?? undefined having an effect. If we want/need to deal with empty strings we could make this || undefined.
There was a problem hiding this comment.
yeah the return type for headers.get() is string | null 👍
packages/bsky/src/util.ts
Outdated
| if (labeler.length === 0) { | ||
| continue | ||
| } |
There was a problem hiding this comment.
This would technically make the header invalid. If we want to pull in a parser, parseList() from structured-headers does what we need here.
There was a problem hiding this comment.
ah nice, that sounds like it's probably worth it 👍
My thinking here was specificlaly for the case of an empty string val ie atproto-accept-labelers:
|
It may be a bit much for now, but it's allowed to combined header values for lists if the header appears multiple times (see here). For example: I could see us just punting on it for now but figured it's worth a note! |
Updated semantics for atproto labelers headers